Cs 10654/invalid url issue present in multiple places#4433
Cs 10654/invalid url issue present in multiple places#4433
Conversation
Fixes 4 critical bugs where prefix URLs and their resolved forms were mixed without proper canonicalization, causing: - Infinite loops in circular reference detection (line 890) - Duplicate resources in results (line 1018) - Broken exclusion filtering (lines 1056-1057) - Non-portable URLs in storage (line 1022-1025) Changes: - Add canonicalURL import from dependency-url - Line 890: Use canonicalURL() when tracking visited resources - Line 1018: Use canonicalURL() when checking omit/included lists - Line 1056-1057: Use canonicalURL() for all ID comparisons - Line 1022-1025: Call unresolveResourceInstanceURLs() before storing All comparisons now properly handle both prefix forms (@cardstack/catalog/foo) and URL forms (http://localhost:4201/catalog/foo) by normalizing them before comparison.
Preview deployments |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e9ab6556c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let isAlreadyOmitted = omit.some((item) => | ||
| item | ||
| ? canonicalURL(item) === canonicalURL(includedResource.id!) | ||
| : false, |
There was a problem hiding this comment.
Guard includedResource.id before canonicalizing link IDs
includedResource.id is forced with ! and passed to canonicalURL(...) before the later includedResource.id presence check, so an id-less linked resource will throw during link loading instead of being skipped. loadLinks handles LooseCardResource | FileMetaResource where id is optional, so stale/partial relationship payloads can trigger this and fail the whole query path; move the canonicalization behind an id guard (or supply a safe fallback) to keep traversal resilient.
Useful? React with 👍 / 👎.
Host Test Results 1 files ± 0 1 suites ±0 2h 30m 45s ⏱️ + 6m 13s Results for commit c8b5b29. ± Comparison against base commit 44d285c. This pull request removes 1 and adds 65 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Pull request overview
Addresses CS-10654 by hardening URL/id handling across runtime-common, host, and base so that prefix-form card/realm IDs (e.g. @cardstack/catalog/...) and absolute URLs can be used interchangeably without triggering Invalid URL failures in common user flows.
Changes:
- Normalize/dedupe card IDs using canonical forms (
canonicalURL,cardIdToURL) across indexing/search aggregation and relationship/link handling. - Replace many
new URL(...)call sites in host/base withcardIdToURL(...)to support prefix IDs in navigation, CRUD, spec preview, listings, commands, and editor flows. - Centralize normalization in
NetworkService.authedFetch, and add regression coverage for GC tail-correlation alias behavior.
Reviewed changes
Copilot reviewed 60 out of 60 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/runtime-common/search-utils.ts | Dedupes included search resources using canonicalURL to treat prefix/absolute IDs as equivalent. |
| packages/runtime-common/realm-index-query-engine.ts | Canonicalizes IDs for dedupe/visited tracking and normalizes relationship/link inclusion for prefix realms. |
| packages/host/tests/unit/garbage-collection-test.ts | Adds regression test ensuring alias resolution doesn’t overwrite saved instance IDs. |
| packages/host/app/services/store.ts | Uses cardIdToURL when constructing realm/path URLs to avoid Invalid URL. |
| packages/host/app/services/recent-files-service.ts | Updates recent-files URL parsing/storage to accept prefix IDs via cardIdToURL. |
| packages/host/app/services/realm-server.ts | Parses realm URLs returned from server via cardIdToURL. |
| packages/host/app/services/operator-mode-state-service.ts | Normalizes many realm/card/code-path URL operations to support prefix IDs (delete, selection, restore, etc.). |
| packages/host/app/services/network.ts | Wraps authedFetch with centralized URL normalization using cardIdToURL. |
| packages/host/app/services/command-service.ts | Uses cardIdToURL for realm resolution from file URLs. |
| packages/host/app/services/ai-assistant-panel-service.ts | Canonicalizes attached card IDs and improves store lookup across prefix/absolute variants. |
| packages/host/app/routes/module.ts | Allows module route IDs/deps to be prefix IDs by using cardIdToURL. |
| packages/host/app/resources/code-diff.ts | Uses cardIdToURL when fetching file source for diffs. |
| packages/host/app/lib/gc-card-store.ts | Prevents overwriting an existing non-local instance id during tail-correlation resolution. |
| packages/host/app/components/operator-mode/workspace-chooser/workspace.gts | Handles published URL parsing/path operations via cardIdToURL. |
| packages/host/app/components/operator-mode/publish-realm-modal.gts | Uses cardIdToURL for realm URL parsing/manipulation and path segment extraction. |
| packages/host/app/components/operator-mode/preview-panel/index.gts | Uses cardIdToURL when navigating to definition modules. |
| packages/host/app/components/operator-mode/overlays.gts | Uses cardIdToURL when constructing navigation targets from string IDs. |
| packages/host/app/components/operator-mode/interact-submode.gts | Uses cardIdToURL for deleting/opening/creating cards from IDs in interact mode. |
| packages/host/app/components/operator-mode/edit-field-modal.gts | Uses cardIdToURL for ref/module/spec URL resolution in field editing flows. |
| packages/host/app/components/operator-mode/create-listing-modal.gts | Allows prefix IDs in realm detection, target realm, and preview navigation. |
| packages/host/app/components/operator-mode/create-file-modal.gts | Normalizes realm/module URLs via cardIdToURL for create/duplicate flows. |
| packages/host/app/components/operator-mode/code-submode/spec-preview.gts | Fixes spec preview URL parsing and relative path handling for prefix realms. |
| packages/host/app/components/operator-mode/code-submode/schema-editor.gts | Uses cardIdToURL when instantiating ModuleSyntax. |
| packages/host/app/components/operator-mode/code-submode/playground/playground-panel.gts | Uses cardIdToURL when deriving realm from selected card IDs. |
| packages/host/app/components/operator-mode/code-submode/module-inspector.gts | Uses cardIdToURL for realm-relative module loading and editor navigation. |
| packages/host/app/components/operator-mode/code-submode/left-panel-toggle.gts | Builds editor/open/download URLs using cardIdToURL for prefix realm URLs. |
| packages/host/app/components/operator-mode/code-submode.gts | Normalizes realm and editor navigation URLs via cardIdToURL throughout code submode. |
| packages/host/app/components/operator-mode/code-editor.gts | Uses cardIdToURL for adoptsFrom/code-ref normalization comparisons. |
| packages/host/app/components/operator-mode/choose-file-modal.gts | Parses known realm URLs via cardIdToURL. |
| packages/host/app/components/operator-mode/card-url-bar.gts | Uses cardIdToURL when user enters URLs in the URL bar. |
| packages/host/app/components/operator-mode/card-schema-editor.gts | Uses cardIdToURL when opening definitions from schema editor. |
| packages/host/app/components/card-search/search-content.gts | Treats prefix IDs as URL-like using cardIdToURL for parsing/realm extraction. |
| packages/host/app/components/ai-assistant/message/attachments.gts | Uses cardIdToURL to safely derive realm/title from error IDs. |
| packages/host/app/commands/write-text-file.ts | Supports prefix/absolute paths and avoids new URL failures via cardIdToURL. |
| packages/host/app/commands/switch-submode.ts | Uses cardIdToURL for code path construction and redirects. |
| packages/host/app/commands/show-file.ts | Uses cardIdToURL for opening files in code mode. |
| packages/host/app/commands/show-card.ts | Uses cardIdToURL for opening card definition modules. |
| packages/host/app/commands/read-text-file.ts | Supports prefix realm/path combinations via cardIdToURL + fallback URL construction. |
| packages/host/app/commands/read-source.ts | Supports prefix realm/path combinations via cardIdToURL + fallback URL construction. |
| packages/host/app/commands/patch-code.ts | Uses cardIdToURL for save/getSource comparisons, lintability checks, and filename derivation. |
| packages/host/app/commands/listing-use.ts | Normalizes realm URL parsing via cardIdToURL for listing operations. |
| packages/host/app/commands/listing-update-specs.ts | Uses cardIdToURL when validating dependency URLs for realm access. |
| packages/host/app/commands/listing-remix.ts | Normalizes realm URL parsing via cardIdToURL for listing remix. |
| packages/host/app/commands/listing-install.ts | Uses cardIdToURL for realm/module source operations and atomic ops realm URL. |
| packages/host/app/commands/listing-create.ts | Uses cardIdToURL for module normalization and realm validation. |
| packages/host/app/commands/instantiate-card.ts | Normalizes module/realm URLs via cardIdToURL before validation and instantiation. |
| packages/host/app/commands/generate-example-cards.ts | Uses cardIdToURL for relativeTo realm parsing during example generation. |
| packages/host/app/commands/evaluate-module.ts | Normalizes module/realm URLs via cardIdToURL before validation. |
| packages/host/app/commands/create-submission-workflow.ts | Uses cardIdToURL when determining workflow realm from listing ID. |
| packages/host/app/commands/create-specs.ts | Uses cardIdToURL when resolving code refs relative to target realms. |
| packages/host/app/commands/copy-source.ts | Uses cardIdToURL for origin/destination source URLs. |
| packages/host/app/commands/copy-file-to-realm.ts | Uses cardIdToURL for source/target realm URL handling and existence checks. |
| packages/host/app/commands/check-correctness.ts | Uses cardIdToURL when parsing target refs and fetching file sources. |
| packages/host/app/commands/add-field-to-card-definition.ts | Uses cardIdToURL for module URL parsing in syntax editing and URL args. |
| packages/base/system-card.gts | Uses cardIdToURL when navigating to active system card. |
| packages/base/skill-set.gts | Uses cardIdToURL when opening skill cards in isolated view. |
| packages/base/menu-items.ts | Uses cardIdToURL when setting code path for menu actions. |
| packages/base/file-menu-items.ts | Uses cardIdToURL when setting code path for file menu actions. |
| packages/base/components/card-list.gts | Uses cardIdToURL for viewCard navigation from list items. |
| packages/base/codemirror-editor.gts | Uses cardIdToURL to safely compute relative card refs from prefix/absolute URLs. |
Comments suppressed due to low confidence (1)
packages/host/app/services/recent-files-service.ts:123
addRecentFile()now storesrealmURLascardIdToURL(currentRealmUrl)(absolute/canonical), but other parts of this service still comparerecentFile.realmURL.hrefagainstoperatorModeStateService.realmURL(a raw string). IfoperatorModeStateService.realmURLcan be a prefix realm id (e.g.@cardstack/catalog/), these comparisons will never match, breaking de-duping/removal/lookup of recent files. Normalize both sides consistently for all realm URL comparisons (e.g. comparecardIdToURL(currentRealmUrl).hreftorecentFile.realmURL.href, and do the same in the index/lookup helpers).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let lastOpenedFile = this.recentFilesService.recentFiles.find( | ||
| (file: RecentFile) => file.realmURL.href === realmUrl, | ||
| ); | ||
| await this.updateCodePath( | ||
| lastOpenedFile | ||
| ? new URL(`${lastOpenedFile.realmURL}${lastOpenedFile.filePath}`) | ||
| : new URL(id), | ||
| ? cardIdToURL(`${lastOpenedFile.realmURL}${lastOpenedFile.filePath}`) | ||
| : cardIdToURL(id), |
| let isAlreadyOmitted = omit.some((item) => | ||
| item | ||
| ? canonicalURL(item) === canonicalURL(includedResource.id!) | ||
| : false, | ||
| ); | ||
| let isAlreadyIncluded = included.some((r) => | ||
| r.id | ||
| ? canonicalURL(r.id) === canonicalURL(includedResource.id!) | ||
| : false, | ||
| ); |
linear: https://linear.app/cardstack/issue/CS-10654/invalid-url-issue-present-in-multiple-places
Problem:
What I experienced in prefix realms (especially catalog):
Failures were spread across connected flows, so one unresolved ID could cascade into multiple breakages:
Scope
This commit addresses widespread
Invalid URLruntime failures caused by mixed usage of:@cardstack/catalog/...)http://...)The fix hardens URL handling across host/base/runtime-common so user actions are stable when prefix realm IDs are involved.
Technical Approach
cardIdToURL(...)for URL operations:network.authedFetchto reduce repeated call-site risk.Outcome
Prefix IDs and absolute URLs now resolve consistently across affected flows, removing major
Invalid URLruntime failure modes and stabilizing user actions.Note
This was intentionally focused on the highest critical paths identified by Codex analysis, primarily in host/base.
There are still lower-priority and edge-case paths to continue auditing in follow-up work.
After the fix
the issues with deletion, the broken spec preview, and ‘adoptsFrom’ not being clickable have been resolved.
Screen.Recording.2026-04-17.at.1.55.53.PM.mov